Skip to content

Conversation

@RHL120
Copy link

@RHL120 RHL120 commented Oct 17, 2022

No description provided.

@Ecco
Copy link
Contributor

Ecco commented Oct 17, 2022

Thanks for this PR! It does fix the build indeed, but I'm not sure this is the right solution.

We would like to avoid installing nwlink, specifically because we want to fix a nwlink version in the repository itself. Indeed, this repository is meant as a template, and we want each app to be able to decide on their own nwlink upgrade path.

Unfortunately, it's a bit annoying here because we need the nwlink command in two different places:

  1. In build.rs to embed the icon
  2. In .cargo/config for the cargo install command

So while we could manually write npx nwlink@x.y.z in both places, that would not be very DRY…

Maybe we should use a package.json file? We managed to avoid having one in the C and C++ sample apps, but we might need one here.

@RHL120
Copy link
Author

RHL120 commented Oct 17, 2022

Thanks for this PR! It does fix the build indeed, but I'm not sure this is the right solution.

We would like to avoid installing nwlink, specifically because we want to fix a nwlink version in the repository itself. Indeed, this repository is meant as a template, and we want each app to be able to decide on their own nwlink upgrade path.

Unfortunately, it's a bit annoying here because we need the nwlink command in two different places:

  1. In build.rs to embed the icon
  2. In .cargo/config for the cargo install command

So while we could manually write npx nwlink@x.y.z in both places, that would not be very DRY…

Maybe we should use a package.json file? We managed to avoid having one in the C and C++ sample apps, but we might need one here.

Hello, I am not a js developer so I know very little about node and I have very little experience in rust. All I know is from reading the rust book and solving the rustlings so pardon me if my question is stupid but is this a valid solution? In build.rs:

    let output = cmd
        .output()
        .or_else(|_| {
            Command::new("npm")
                .args(&["install", "-g", "nwlink"])
                .output()
                .and_then(|_| cmd.output())
        })
        .expect("You should install npm and nwlink");

I think this should work because as far as I can tell, build runs before install

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants